feat: format SharedDB operations as human-readable list of changes #2877
Conversation
|
Removed vultr server and associated DNS entries |
…ss/formatted-operations
…ss/formatted-operations
…ss/formatted-operations
| ]; | ||
|
|
||
| expect(formatOps(flowWithChecklist, ops)).toEqual([ | ||
| 'Updated Checklist text from "Which fruits?" to "Which vegetables?"', |
There was a problem hiding this comment.
It feels like helpful context to print the value of the updated data prop here, but it's a big difference between showing how text or title change than it is, for example, help text.
We may want to consider making a sort of "black list" of data props where we'd only show the name - eg "Updated Checklist moreInfo"- without the values? Could also possibly address via UI design by nesting changes list within accordion drawers in side panel or similar?
I plan to leave in as-is initially for explicitness, but would expect this feedback to come up pretty early in testing !
There was a problem hiding this comment.
This sounds sensible - a mapping of fields which do and don't get their values displayed makes sense.
| }); | ||
|
|
||
| return output; | ||
| }; |
There was a problem hiding this comment.
Future scope:
- Live edits are tracked via operations, which this formatter "translates" into something human readable
- Publishing changes are summarised via a JSON diff of two flow structures, which we summarise as
alteredNodes- currently this may not perfectly match up to the summarised operations - For more consistency in the future between "edit history" & "publishing changes", our publishing JSON diff could first be mapped to a JSON OT using a package like this https://github.com/kbadk/json0-ot-diff - then both could share this tranlator
Don't think this is necesary just yet as recent changes to publishing are testing well, but just putting it on our radar !
jessicamcinchak
changed the title
| * Translates a list of ShareDB operations into a human-readable change summary. | ||
| * See https://github.com/ottypes/json0?tab=readme-ov-file#summary-of-operations | ||
| */ | ||
| export const formatOps = (graph: Graph, ops: any[]): string[] => { |
There was a problem hiding this comment.
| export const formatOps = (graph: Graph, ops: any[]): string[] => { | |
| export const formatOps = (graph: Graph, ops: OT.Op[]): string[] => { |
Not totally sure on types here - is this right?
There was a problem hiding this comment.
Yes - I got this mostly working - OT.Op is a big union type, so individual handleAdd() etc methods need to do a bit of casting like op as Op.Object.Add to match their semantic meaning. Look okay?
There was a problem hiding this comment.
See comment below about type guards using is. I think under the hood it's essentially the same thing, but wanted to raise as an option.
| ]; | ||
|
|
||
| expect(formatOps(flowWithChecklist, ops)).toEqual([ | ||
| 'Updated Checklist text from "Which fruits?" to "Which vegetables?"', |
There was a problem hiding this comment.
This sounds sensible - a mapping of fields which do and don't get their values displayed makes sense.
| } | ||
| const operationTypes = Object.keys(op); | ||
|
|
||
| if (operationTypes.includes("od") && operationTypes.includes("oi")) { |
There was a problem hiding this comment.
nit: If we made this a type guard, we could avoid the casting here if we wanted to -
const isUpdate = (op: OT.Op): op is OT.Object.Replace => {
const operationTypes = Object.keys(op);
return operationTypes.includes("od") && operationTypes.includes("oi");
}There was a problem hiding this comment.
This is super useful to keep in mind & I'll definitley come back to it - will personally feel more confident full type-guarding after more testing of various operation scenarios !
Thinking about how we'll display recent operations in the "undo" dialog.
Our ShareDB operations are formatted according to Version 0 of the JSON OT type as described here https://github.com/ottypes/json0?tab=readme-ov-file#summary-of-operations
I was really hoping to find a library to handle pretty-printing & parsing for us, but no such luck so I've started writing our own translator. Lots of room for improvement here, but it's a baseline we can iterate from & keep thinking on for now?
To test please run
window.featureFlags.toggle("UNDO")in your console first !Todo:
formatOps()